Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Fixes order in which default initializers are injected #119

Merged
merged 7 commits into from
Sep 14, 2016

Conversation

weierophinney
Copy link
Member

Uses the test from #117 to find a solution for #100 by overriding configure() to first remove the default initializers from the initializer stack if present, and then push them on in the appropriate positions once configuration is complete.

Slamdunk and others added 3 commits September 13, 2016 10:04
Reproduce issue zendframework#100 without mvc and modulemanager dependencies
Uses the test from zendframework#117 to find a solution for zendframework#100 by overriding `configure()`
to first remove the default initializers from the initializer stack if present,
and then push them on in the appropriate positions once configuration is
complete.
@weierophinney weierophinney added this to the 2.9.1 milestone Sep 13, 2016
use Zend\ServiceManager\InitializerInterface;
use Zend\ServiceManager\ServiceLocatorInterface;

class DependancyInitializer implements InitializerInterface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependacy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remembered to fix that too late; have it fixed locally, and will include in the merge if the rest looks good. (That came from the PR that provided the tests! w00t for PRs with tests!)

- Ensure consistent license on new files, with correct year
- `s/Dependancy/Dependency/g`
- Added dockblocks on new class methods and properties
Removes need for additional test asset classes by using mock objects to
test behavior of initializers.

Adds a test to verify that the plugin manager injected into a form's
factory is the application form element manager.
@weierophinney
Copy link
Member Author

Marking as a WIP, as I'm refactoring the tests to use mock objects as spies, and writing a test to verify the secondary behavior of ensuring that the callElementInit() initializer triggers after the injectFactory() initializer.

<?php
/**
* @link http://github.com/zendframework/zend-servicemanager for the canonical source repository
* @copyright Copyright (c) 2016 Zend Technologies USA Inc. (http://www.zend.com)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyright wording inconsistency

Adds a unit test to ensure that the `injectFactory()` initializer is
injected prior to the `callElementInit()` initializer, and updates the
V2 polyfill to ensure the behavior is correct.

A new test asset was created to help spy on behavior.
@weierophinney weierophinney removed the WIP label Sep 14, 2016
@weierophinney
Copy link
Member Author

Test covering the behavior "injectFactory() initializer should be invoked prior tocallElementInit()` initializer" is now written. In writing it, I discovered a discrepancy in the V2 polyfill and corrected it. Ready for final review; pinging @Ocramius!

$formElementManagerConfig = new Config([
'factories' => [
'MyForm' => function () {
return new TestAsset\Form();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ocramius Had to use a test asset here. When I tried mocking, I was hitting out-of-memory errors. Worked fine for the initializers, but not for the form.

@weierophinney weierophinney merged commit b839697 into zendframework:master Sep 14, 2016
weierophinney added a commit that referenced this pull request Sep 14, 2016
weierophinney added a commit that referenced this pull request Sep 14, 2016
weierophinney added a commit that referenced this pull request Sep 14, 2016
@weierophinney weierophinney deleted the hotfix/117 branch September 14, 2016 16:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants